refactor!: own the interpreter runtime toolchain rules instead of rules_python's#1305
refactor!: own the interpreter runtime toolchain rules instead of rules_python's#1305jbedard wants to merge 1 commit into
Conversation
✨ Aspect Workflows Tasks📅 Fri Jul 17 08:12:26 UTC 2026 ✅ 40 successful tasks
⏱ Last updated Fri Jul 17 08:15:52 UTC 2026 · 📊 GitHub API quota 5,519/15,000 (37% used, resets in 13m) |
py_binary startup benchmark
sys.path quality
Bazel analysis benchmark
Auxiliary metrics
|
55d4a06 to
2531a57
Compare
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
There are three compatibility/correctness gaps in the refreshed runtime boundary: the emitted runtime provider breaks rules_python zipapp/interpreter composition, the new private exec-toolchain type breaks modules that provision only a rules_python interpreter but consume rules_py uv wheels, and the highest-version fallback can emit unusable pyc for a different target Python version. The current interop coverage does not exercise these paths. The PR description also appears stale: it says the exec-toolchain type is unchanged while the code changes it, and says rules_python binaries cannot execute while the new coverage runs one.
| # The //py/private/toolchain:exec_tools_toolchain_type contract. | ||
| exec_runtime = runtime, | ||
| ), | ||
| DefaultInfo(files = depset([ctx.file.interpreter], transitive = [runtime.files])), |
There was a problem hiding this comment.
The runtime returned through the standard Python toolchain now carries rules_py's distinct PyRuntimeInfo. A rules_python py_binary/py_test forwards that runtime object, and rules_python py_zipapp_binary/py_zipapp_test (and explicit py_interpreter(binary=...)) require and index rules_python's PyRuntimeInfo, so those compositions fail analysis even though the binary itself can run. Could this retain/publish the rules_python runtime provider on the standard-toolchain path and add an e2e zipapp/interpreter consumer over a rules_python binary using the provisioned runtime?
There was a problem hiding this comment.
Why would "A rules_python py_binary/py_test" ever receive "rules_py's distinct PyRuntimeInfo"?
There was a problem hiding this comment.
Because both rule sets resolve the same standard toolchain type. In rules_python 1.9.0, @rules_python//python:toolchain_type aliases @bazel_tools//tools/python:toolchain_type; this PR registers the provisioned runtime against that type and puts its new provider in ToolchainInfo.py3_runtime. The existing rules-python-consumers:pycowsay fixture already exercises a rules_python binary landing on that runtime.
rules_python's executable rule field-reads py3_runtime, so the binary itself works, then forwards that same provider object. Adding py_zipapp_binary(name = "app_zip", binary = ":pycowsay") fails because zipapp requires and indexes rules_python's PyRuntimeInfo, not the structurally compatible rules_py provider. py_interpreter(binary = ...) has the same identity check. That is the composition the current fixture misses.
There was a problem hiding this comment.
Should we drop the use of @bazel_tools//tools/python:toolchain_type as well then?
There was a problem hiding this comment.
I would keep the standard target type. @bazel_tools//tools/python:toolchain_type is the shared target-runtime contract, not a rules_python implementation detail: third-party/system/FIPS runtimes and both rule sets use it. Moving rules_py to a private target type would make mixed builds select two independent runtimes and would break the currently supported direction where rules_py consumes a rules_python-provisioned interpreter.
The smaller option is to keep the shared type and use/re-export rules_python's public @rules_python//python:py_runtime_info.bzl provider for ToolchainInfo.py3_runtime. That still removes the py_runtime + py_runtime_pair + py_exec_tools_toolchain rule stack while preserving provider identity for zipapp/interpreter composition; one generated runtime target can serve both registrations. A fully private target type is coherent only as an explicit full-detachment/major-version migration, with adapters for external runtimes, and is materially broader than this load-reduction refactor.
There was a problem hiding this comment.
The new head correctly re-exports rules_python's public PyRuntimeInfo and places that exact provider in the shared toolchain, which fixes the composition bug. There is still no py_zipapp_binary/py_zipapp_test or explicit-interpreter consumer in the e2e suite: pycowsay only runs the underlying binary. Please add the small zipapp regression so provider identity cannot silently regress.
|
|
||
| PY_TOOLCHAIN = "@bazel_tools//tools/python:toolchain_type" | ||
| EXEC_TOOLS_TOOLCHAIN = "@rules_python//python:exec_tools_toolchain_type" | ||
| EXEC_TOOLS_TOOLCHAIN = "@aspect_rules_py//py/private/toolchain:exec_tools_toolchain_type" |
There was a problem hiding this comment.
Changing the exec-tools type to a new rules_py-private type breaks the supported mixed setup where a module provisions only rules_python's python.toolchain() and consumes rules_py uv dependencies: WhlInstall/py_unpacked_wheel/site-merge now have no matching exec toolchain. The rules-python-interop case provisions a rules_python runtime but only exercises pip.parse, so it misses this. Please keep/bridge the existing exec-tools type or register an adapter automatically, and extend that interop case with a rules_py uv wheel (and ideally a site-merge collision).
There was a problem hiding this comment.
The new rules-python-interop case now provisions rules_python's target runtime, consumes a real rules_py uv/cowsay wheel through the bundled exec fallback, and passes on Bazel 8/9. That covers the mixed setup from this thread; resolving. Small documentation correction: the new comments call the fallback an adapter bridging rules_python exec tools, but the implementation is the bundled PBS exec fallback.
| name = "zz_{name}_exec_tools_fallback", | ||
| exec_compatible_with = {exec_compatible_with}, | ||
| toolchain = "@{repo}//:runtime", | ||
| toolchain_type = "@aspect_rules_py//py/private/toolchain:exec_tools_toolchain_type", |
There was a problem hiding this comment.
Falling back to the highest provisioned exec interpreter when the active target version is absent is unsafe for compile_pyc. WhlInstall takes the target layout/version from the standard toolchain but runs compileall with this exec runtime; a 3.12 target plus a 3.13-only rules_py hub therefore installs 3.13-magic pyc under python3.12. Require a matching exec version (or explicitly disable/error on pyc compilation when none exists) and add a mixed-version compile_pyc regression.
There was a problem hiding this comment.
The new major/minor guard in whl_install/rule.bzl correctly disables compileall when target and exec runtimes differ, and the matching/mismatching action-argv tests exercise both paths. That addresses the wrong-magic .pyc failure; resolving.
2531a57 to
5cda78b
Compare
jbedard
changed the title
78fe5e9 to
329bc54
Compare
jbedard
changed the title
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
The new head fixes and meaningfully covers mixed provisioning and target/exec bytecode-version mismatch. The provider identity is now correct, but please retain the small zipapp regression requested in the remaining thread. The PR description/release notes also need a refresh: they still describe a distinct rules_py PyRuntimeInfo, no rules_python minimum/API dependency, and rules_python-only setups failing, while the new code and tests now deliberately do the opposite.
|
All 88 checks are green. The mixed-provisioning and target/exec bytecode-mismatch threads are fixed and covered, but GitHub does not permit an external reviewer to resolve them; could a maintainer resolve those two? The remaining request is the small zipapp/provider-identity regression plus updating the now-stale release notes. |
…es_python's Generated interpreter repos now instantiate a single rules_py py_runtime_toolchain target: one @python_X//:runtime replaces the rules_python py_runtime + py_runtime_pair + py_exec_tools_toolchain stack, backing both toolchain registrations. The payload remains rules_python's public PyRuntimeInfo (re-exported from py/private/interpreter/runtime.bzl and @aspect_rules_py//py:defs.bzl): @bazel_tools//tools/python:toolchain_type is the shared target-runtime contract, and preserving provider identity keeps interop working in both directions — rules_py rules consume rules_python-provisioned runtimes (e.g. system interpreters), and rules_python-defined executables and their downstream consumers (py_console_script_binary, py_zipapp_binary, py_interpreter) analyze and run on rules_py-provisioned toolchains. Each interpreter repo's rules_python loads shrink from 31 .bzl files to the py_cc_toolchain chain plus the two-file py_runtime_info.bzl provider definition, insulating provisioned toolchains from rules_python rule implementation churn; the runtime layer no longer pins any rules_python minimum version. Exec-tools build actions (whl_install, py_unpacked_wheel, py_venv site-package merging) now resolve a rules_py-owned toolchain type, //py/private/toolchain:exec_tools_toolchain_type. rules_py registers nothing under rules_python's exec-tools type, leaving that namespace — including precompiling — entirely to rules_python. Registrations are version-gated so the exec interpreter follows the Python version flags (#1309), falling back to the platform's highest provisioned version for unmatched configurations; the hub registration bundled in rules_py's own MODULE means this resolves in every consumer, including modules that provision interpreters only through rules_python's python.toolchain() (pinned by e2e uv_deps_test). whl_install now emits --compile-pyc only when the exec runtime's major.minor matches the target runtime's: a fallback exec interpreter of another version would write bytecode whose magic is wrong for the lib/python{version}/ layout. This was latent before this change as well, since rules_python's exec-tools sentinel resolves the default version in the exec configuration regardless of the target version. Covered by a matched/mismatched analysis-test pair. Breaking change: generated interpreter repo targets are renamed — @python_X//:py3_runtime, :runtime_pair, and :exec_tools_toolchain are now a single @python_X//:runtime. Only direct label references need updating; toolchain resolution and provider identity are unaffected.
329bc54 to
d3a616f
Compare
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
The zipapp regression and updated runtime documentation address the prior provider-identity request. One bytecode-compatibility gap remains in the precompile guard.
| # bytecode magic would be wrong for the lib/python{version}/ layout. | ||
| exec_matches_target = ( | ||
| exec_runtime.interpreter_version_info.major == py_toolchain.interpreter_version_info.major and | ||
| exec_runtime.interpreter_version_info.minor == py_toolchain.interpreter_version_info.minor |
There was a problem hiding this comment.
Comparing only major/minor is not sufficient for bytecode compatibility. CPython changes the magic number between prereleases of the same minor; this is a supported mixed-provisioning case here because rules_python 1.9.0 can select 3.15.0a2 while rules_py provisions 3.15.0a6. Both report 3.15, but their magic numbers are 3656 and 3660, so this guard enables compileall and produces unusable pyc files for the target runtime. Please require the full interpreter_version_info (including release level/serial and micro) to match, and add a same-minor prerelease-mismatch regression.
2 participants
Generated interpreter repos now instantiate a single rules_py
py_runtime_toolchain target: one @python_X//:runtime replaces the
rules_python py_runtime + py_runtime_pair + py_exec_tools_toolchain stack,
backing both toolchain registrations. The payload remains rules_python's
public PyRuntimeInfo (re-exported from py/private/interpreter/runtime.bzl and
@aspect_rules_py//py:defs.bzl): @bazel_tools//tools/python:toolchain_type is
the shared target-runtime contract, and preserving provider identity keeps
interop working in both directions — rules_py rules consume
rules_python-provisioned runtimes (e.g. system interpreters), and
rules_python-defined executables and their downstream consumers
(py_console_script_binary, py_zipapp_binary, py_interpreter) analyze and run
on rules_py-provisioned toolchains.
Each interpreter repo's rules_python loads shrink from 31 .bzl files to the
py_cc_toolchain chain plus the two-file py_runtime_info.bzl provider
definition, insulating provisioned toolchains from rules_python rule
implementation churn; the runtime layer no longer pins any rules_python
minimum version.
Exec-tools build actions (whl_install, py_unpacked_wheel, py_venv
site-package merging) now resolve a rules_py-owned toolchain type,
//py/private/toolchain:exec_tools_toolchain_type. rules_py registers nothing
under rules_python's exec-tools type, leaving that namespace — including
precompiling — entirely to rules_python. Registrations are version-gated so
the exec interpreter follows the Python version flags (#1309), falling back
to the platform's highest provisioned version for unmatched configurations;
the hub registration bundled in rules_py's own MODULE means this resolves in
every consumer, including modules that provision interpreters only through
rules_python's python.toolchain() (pinned by e2e uv_deps_test).
whl_install now emits --compile-pyc only when the exec runtime's
major.minor matches the target runtime's: a fallback exec interpreter of
another version would write bytecode whose magic is wrong for the
lib/python{version}/ layout. This was latent before this change as well,
since rules_python's exec-tools sentinel resolves the default version in the
exec configuration regardless of the target version. Covered by a
matched/mismatched analysis-test pair.
Changes are visible to end-users: yes
Breaking change: generated interpreter repo targets are renamed —
@python_X//:py3_runtime, :runtime_pair, and :exec_tools_toolchain are now a
single @python_X//:runtime. Only direct label references need updating;
toolchain resolution and provider identity are unaffected.
Test plan